Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: apply goimports linter #332

Merged
merged 1 commit into from
Dec 6, 2024

Conversation

G-Rath
Copy link
Collaborator

@G-Rath G-Rath commented Dec 5, 2024

This enables the goimports linter to ensure our imports are consistently and well formatted, which most already are

Relates to #274

@G-Rath G-Rath mentioned this pull request Dec 5, 2024
46 tasks
"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
"github.com/google/go-containerregistry/pkg/v1"
v1 "github.com/google/go-containerregistry/pkg/v1"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not quite sure if I understand why it's better formatting to make things ending with "v1" a named import?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was curious about that myself but unfortunately I couldn't find a concrete laymen answer, just the issue that lead to it being a thing.

From what I could tell, something has an issue with implicitly-named imports of versions like this maybe because of special handling Go has for major versions of packages, and that resulted in something erroring - I think it might be a technical edge case in goimports rather than an actual language constraint, and that they decided it was fine to just make the convention to explicitly define the package name in this case?

@copybara-service copybara-service bot merged commit da47694 into google:main Dec 6, 2024
7 checks passed
@G-Rath G-Rath deleted the linting/enable-goimports branch December 11, 2024 18:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants